Skip to content

Upgrade to Spring Boot 2.7.18, migrate off WebSecurityConfigurerAdapter, add JaCoCo coverage - #848

Open
devin-ai-integration[bot] wants to merge 1 commit into
masterfrom
devin/1784918000-spring-boot-2.7-jacoco
Open

devin-ai-integration[bot] wants to merge 1 commit into
masterfrom
devin/1784918000-spring-boot-2.7-jacoco

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Jul 23, 2026

Copy link
Copy Markdown

Summary

  • Bumps org.springframework.boot plugin 2.6.3 → 2.7.18 (dependency-management, Java 11 targets, and pinned MyBatis/DGS/rest-assured deps unchanged).
  • Migrates WebSecurityConfig off the deprecated WebSecurityConfigurerAdapter (removed in Spring Security 6): configure(HttpSecurity)@Bean SecurityFilterChain securityFilterChain(HttpSecurity) returning http.build(). All rules preserved verbatim (CSRF off, CORS, 401 entry point, stateless sessions, same antMatchers chain, JwtTokenFilter before UsernamePasswordAuthenticationFilter). No web-ignoring config existed, so no WebSecurityCustomizer was needed.
  • Adds the jacoco plugin: test finalizedBy jacocoTestReport (XML+HTML), DGS-generated classes (io/spring/graphql/types/**, DgsConstants*) excluded from counting, and jacocoTestCoverageVerification wired into check with a 0.50 line-coverage minimum. Note: the actual baseline is 0.53 line coverage (not 0.70 as initially assumed) — the uncovered code is almost entirely the handwritten GraphQL datafetchers in io/spring/graphql (0.02 covered). Threshold set to 0.50 so the build enforces no regression; raising it requires adding DGS datafetcher tests.
  • Adds WebSecurityConfigTest verifying the refactored auth rules (permitAll paths reachable without a token, /articles/feed and other protected endpoints return 401 without/with-invalid token, pass with a valid JWT).
  • Adds gradle.properties with --add-exports jdk.compiler/... JVM args so spotless/google-java-format works when building on JDK 16+ (no effect on JDK 11).
  • Adds a GitHub Actions workflow (build.yml) running ./gradlew build (tests + coverage verification) on JDK 11 and uploading the JaCoCo report. A Gradle 7.4 wrapper was already committed, so none was added.

Verified locally: full ./gradlew build passes — 76 tests, coverage verification OK.

Link to Devin session: https://app.devin.ai/sessions/9b4f41de2cd34f90bdc4fbef47513c2f


Devin Review

Status Commit
⚪ Not started

Run Devin Review

Open in Devin Review (Staging)
Open in Devin Review

@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

Open in Devin Review

Comment on lines 25 to 26
@Bean
public JwtTokenFilter jwtTokenFilter() {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: JwtTokenFilter registered both as security filter and servlet filter (pre-existing)

jwtTokenFilter() is declared as a @Bean of a Filter type (src/main/java/io/spring/api/security/WebSecurityConfig.java:26-28) and also inserted into the security chain via addFilterBefore (:64). Spring Boot auto-detects Filter beans and registers them with the servlet container for all requests, so this filter effectively runs outside the security chain too. This was already the case before the migration and does not cause double execution because JwtTokenFilter extends OncePerRequestFilter, so it is not a regression. Noting it here in case future work assumes the filter only runs where the security matchers apply.

(Refers to lines 25-28)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct — this dual registration is pre-existing behavior carried over unchanged from the adapter-based config, and OncePerRequestFilter prevents double execution. Leaving as-is to keep the migration behavior-preserving; suppressing the servlet-container registration (via a FilterRegistrationBean with setEnabled(false)) could be done as a follow-up if desired.

Comment thread build.gradle
Comment on lines +87 to +95
violationRules {
rule {
limit {
counter = 'LINE'
value = 'COVEREDRATIO'
minimum = 0.50
}
}
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: JaCoCo line-coverage threshold set well below actual baseline

The coverage gate is set to minimum = 0.50 (build.gradle:92) while the PR description states the actual baseline is ~0.53. This leaves ~3 percentage points of slack, meaning a meaningful coverage regression could pass the build unnoticed. Not a correctness bug, but worth confirming the threshold intentionally trails the baseline rather than tracking it.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intentional — the 0.50 threshold trails the 0.53 baseline slightly so the gate doesn't flake on small legitimate refactors while still catching meaningful regressions. Happy to tighten it (or raise it after adding GraphQL datafetcher tests) if a stricter gate is preferred.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants